linux_android_with_fallback: detect getrandom #758

tamird · 2025-11-24T19:50:43Z

Avoid dlsym when statically linking crt.

Relative to #598 this PR keeps dlsym in place when linking dynamically and avoids the dependency on a C compiler. The benefits are as before: avoiding an indirect call when linking statically.

This also contains minor cleanup in src/backends/netbsd.rs.

Running CI on any branch is more ergonomic, particularly for folks using CI in their forks.

Avoid dlsym when statically linking crt.

newpavlov

I don't think I like the proposed approach. Not only it's significantly more complex, but I am also not sure about its universality. IIUC it checks that getrandom is available on local machine, but compiled binary may be executed in a completely different environment!

Until the minimum supported Linux kernel version is bumped by Rust, I don't think we should statically link to the getrandom symbol.

newpavlov · 2025-11-24T20:30:40Z

.github/workflows/build.yml


 on:
  push:
-    branches: master


These changes are not relevant.

Sure, they're in a separate commit with reasoning given (it's a bummer you guys insist on squashing on merge).

@tamird I think this is a reasonable thing to do, but changing on which branches CI runs is tricky from a security perspective, and it deserves a dedicated PR.

newpavlov · 2025-11-24T20:35:25Z

Also, it's better to move the NetBSD cleanup into a separate PR.

newpavlov · 2025-11-24T20:40:34Z

And don't forget that we have getrandom_backend="linux_getrandom", if you are fine with bumping minimum supported Linux kernel version for your application.

tamird · 2025-11-24T20:49:50Z

I don't think I like the proposed approach. Not only it's significantly more complex, but I am also not sure about its universality. IIUC it checks that getrandom is available on local machine, but compiled binary may be executed in a completely different environment!

Until the minimum supported Linux kernel version is bumped by Rust, I don't think we should statically link to the getrandom symbol.

Hm. I don't think I follow you here - we're checking if getrandom is available statically on the target, i.e. if we're statically linking against MUSL then getrandom is available statically. But we still check at runtime that the target kernel allows it.

newpavlov · 2025-11-24T20:54:34Z

In my understanding, you check that you can link statically on your host, so your build script will happily link to glibc's getrandom (to which your application links implicitly when you compile for GNU Linux targets) and will erroneously report that static linking can be used just fine, while the resulting application could be then deployed on a system with old glibc and Linux kernel.

Even if I misunderstand something and it works correctly in all corner cases, I don't think the additional complexity is justified by the arguably very minor benefit.

newpavlov · 2025-11-24T20:56:59Z

src/backends/netbsd.rs

+    static GETRANDOM_FN: lazy::LazyUsize = lazy::LazyUsize::new();
+
+    let fptr = GETRANDOM_FN.unsync_init(init);
+    let fptr = unsafe { mem::transmute::<usize, GetRandomFn>(fptr) };


Note that this is probably incorrect from the strict pointer provenance point of view.

I don't think so. The provenance of the original pointer is the same as the provenance of the pointer constructed here.

tamird · 2025-11-24T21:14:07Z

In my understanding, you check that you can link statically on your host, so your build script will happily link to glibc's getrandom (to which your application links implicitly when you compile for GNU Linux targets) and will erroneously report that static linking can be used just fine, while the resulting application could be then deployed on a system with old glibc and Linux kernel.

I don't understand this. If static linking is used, what does it matter what libc is on the target that is deployed to? Yes, the kernel might not support getrandom, but that is why we still check it at runtime using is_getrandom_good.

Even if I misunderstand something and it works correctly in all corner cases, I don't think the additional complexity is justified by the arguably very minor benefit.

Well, that's insurmountable. If you are not open to persuasion, then there's nothing to discuss.

tamird · 2025-11-24T21:23:08Z

Also, it's better to move the NetBSD cleanup into a separate PR.

Moved to #759.

josephlr

Can you remove the CI changes the changes in #759 to make it easier to actually see what is changing with linux_android_with_fallback?

josephlr · 2025-11-24T21:55:11Z

.github/workflows/build.yml


 on:
  push:
-    branches: master


@tamird I think this is a reasonable thing to do, but changing on which branches CI runs is tricky from a security perspective, and it deserves a dedicated PR.

newpavlov · 2025-11-24T23:48:20Z

I missed the crt-static part. In the build script you run the linking check only on targets which link to the C runtime statically. In that case this could probably work, but I still do not like the heaps of additional complexity, especially considering that we already statically link to getrandom on MUSL targets. I think the linux_getrandom opt-in backend is sufficient for users who want to enforce getrandom linking on non-MUSL statically linked targets (niche of the niche by itself).

So unless @josephlr has a different opinion, I think we should not use this approach.

briansmith · 2025-12-05T17:17:31Z

build.rs

    }
+
+    if cfg!(target_feature = "crt-static") {
+        match std::process::Command::new(std::env::var_os("RUSTC").unwrap())


I don't think this kind of autoconf-like detection is good to introduce as autoconf-like configuration is antithetical to how Rust projects' build systems work.

Sounds dogmatic. The real thing we want is a weak symbol but rust-lang/rust#29603.

briansmith · 2025-12-05T17:40:24Z

Is static linkking to libc on Android supported by Android itself? It seems like it was decided to not try to support it in Cargo: rust-lang/cargo#10765. Go project has a similar concern: golang/go#59942 (comment). It seems like maybe it can be done and is needed for some internal stuff in Android but I am not sure that an APK-packaged app can do it.

I remember we had a discussion about whether we could issue syscalls directly on Android, and we decided to defer them to bionic because Android prefers that. But if we're linking bionic statically then we'd be effectively back to issuing syscalls directly.

tamird · 2025-12-05T18:14:44Z

This PR doesn't change behavior for android, it changes behavior for linux musl.

briansmith · 2025-12-05T18:33:55Z

build.rs

        println!("cargo:rustc-cfg=getrandom_msan");
    }
+
+    if cfg!(target_feature = "crt-static") {


I don't think it is good to have such auto-detection at all, but especially we shouldn't do it based on only whether crt-static is enabled. If the goal is to do something specific for MUSL on Linux. then we should restrict it to that. And, we should add a comment about why we are doing this.

Though, again, I don't think we should.

Further, if the goal is to just verify that getrandom() is available and linking succeeds on statically-linked MUSL on Linux, then it is already guaranteed to succeed because MUSL 1.2.3 is the minimum MUSL version since MSRV 1.71 for Linux targets.

The goal is to avoid drive this based on the user's request to statically link crt rather than e.g. hardcoding musl.

The goal is to avoid drive this based on the user's request to statically link crt rather than e.g. hardcoding musl.

Why would it make sense to dynamically set rustc-cfg=has_libc_getrandom on Windows (or any other non-Linux target)?

In general, the end-user will know whether to set has_libc_getrandom or not, and they can do it in their build system. And they could implement the dynamic detection in their build system, if they are into such things.

Anyway, I think we can close this PR now.

The point is that the user may just say "i want static linking" and expect appropriate choices to be made by her dependencies.

briansmith · 2025-12-05T19:10:31Z

This PR doesn't change behavior for android, it changes behavior for linux musl.

The PR description says "Avoid dlsym when statically linking crt." But, we have:

    #[cfg(target_env = "musl")]
    let raw_ptr = {
        let fptr: GetRandomFn = libc::getrandom;
        unsafe { transmute::<GetRandomFn, *mut c_void>(fptr) }
    };

I.e. on Linux MUSL we already avoid DLSYM.

newpavlov · 2025-12-10T09:17:28Z

Closing per the comments above.

briansmith · 2025-12-10T18:43:19Z

The PR description says "Avoid dlsym when statically linking crt." But, we have:
>  #[cfg(target_env = "musl")]

Possibly the intent is to support static linking of a libc that isn't musl, bionic, glibc, or uclibc, namely LLVM-libc or whatever it is called. I will suggest a solution in a new issue.

briansmith · 2025-12-10T19:00:27Z

Possibly the intent is to support static linking of a libc that isn't musl, bionic, glibc, or uclibc, namely LLVM-libc or whatever it is called. I will suggest a solution in a new issue.

Now it's issue #766.

.github: run CI on all branches

3bd0ebd

Running CI on any branch is more ergonomic, particularly for folks using CI in their forks.

tamird force-pushed the dlsym-fallback-static branch from de32ca5 to 88baf82 Compare November 24, 2025 19:55

linux_android_with_fallback: detect getrandom

edca8cf

Avoid dlsym when statically linking crt.

tamird force-pushed the dlsym-fallback-static branch from 88baf82 to edca8cf Compare November 24, 2025 19:58

newpavlov reviewed Nov 24, 2025

View reviewed changes

josephlr requested changes Nov 24, 2025

View reviewed changes

josephlr mentioned this pull request Nov 24, 2025

Simplify atomics using lazy #759

Closed

briansmith reviewed Dec 5, 2025

View reviewed changes

newpavlov closed this Dec 10, 2025

linux_android_with_fallback: detect getrandom #758

linux_android_with_fallback: detect getrandom #758

Conversation

tamird commented Nov 24, 2025

Uh oh!

newpavlov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

newpavlov commented Nov 24, 2025

Uh oh!

newpavlov commented Nov 24, 2025

Uh oh!

tamird commented Nov 24, 2025

Uh oh!

newpavlov commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tamird commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tamird commented Nov 24, 2025

Uh oh!

josephlr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

newpavlov commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith commented Dec 5, 2025

Uh oh!

tamird commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith commented Dec 5, 2025

Uh oh!

newpavlov commented Dec 10, 2025

Uh oh!

briansmith commented Dec 10, 2025

Uh oh!

briansmith commented Dec 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

newpavlov left a comment •

edited

Loading

newpavlov commented Nov 24, 2025 •

edited

Loading

tamird commented Nov 24, 2025 •

edited

Loading

josephlr left a comment •

edited

Loading

newpavlov commented Nov 24, 2025 •

edited

Loading